Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improved the agora detector #3360

Merged

Conversation

kashifkhan0771
Copy link
Contributor

@kashifkhan0771 kashifkhan0771 commented Oct 3, 2024

Description:

This PR improve the agora detector logic to avoid processing when secret and key matches as both have same regex.
Pattern Test Cases Output:
agora-detector-tests

Checklist:

  • Tests passing (make test-community)?
  • Lint passing (make lint this requires golangci-lint)?

@kashifkhan0771
Copy link
Contributor Author

Changes:

  • Updated the logic to avoid processing same secret and key
  • Renamed few variables for better readability and understanding
  • Updated Pattern test cases accordingly. You can see in the output of test cases that it now process only different key and secret.

@kashifkhan0771
Copy link
Contributor Author

Question: Currently, both the key and secret regex patterns include the agora prefix, which expects the keyword 'agora' to appear near both the key and secret in the data. This helps reduce false positives by narrowing the search. However, it's more likely that the keyword will be present near either the key or the secret, but not both. Removing the prefix would allow scanning the data more broadly for either the key or secret, but it could also increase false positives, as any 32-character alphanumeric string could match.

I need some suggestions here 😃

@rgmz
Copy link
Contributor

rgmz commented Oct 3, 2024

My advice: use GitHub search to find real-world examples and use them as inspiration for unit test cases.

@bugbaba
Copy link

bugbaba commented Oct 4, 2024

@kashifkhan0771 My advice would be to remove those and run against a good number of projects in the wild. Then analyze the false positives and try to observe if there is any pattern in it.

Personally I spent most of time analyzing the false positives and false negatives :)

This is what I am using personally currently
image

@rgmz
Copy link
Contributor

rgmz commented Oct 4, 2024

This is what I am using personally currently

I like key/token/secret prefixes. I find that not using the keyword as a prefix tends to find more results. The keyword already guarantees the presence of that word in the chunk.

e.g., this is a common pattern that doesn't get detected if you only use detectors.PrefixRegex([]string{"agora"}).

KEY=abdcefg...
SECRET=abdcefg...
curl "https://api.agora.com/" -u "$KEY:$SECRET" ...

@kashifkhan0771
Copy link
Contributor Author

kashifkhan0771 commented Oct 7, 2024

Thanks @bugbaba and @rgmz for the suggestions! I updated the keywords, and now it also correctly pick key and secrets separately because of different keywords. I kept the check for the same key and secret as a precaution.

@zricethezav zricethezav merged commit 23e8ae4 into trufflesecurity:main Oct 7, 2024
12 checks passed
abmussani added a commit to abmussani/trufflehog that referenced this pull request Oct 9, 2024
* main: (79 commits)
  Log skipped files on debug level (trufflesecurity#3383)
  build: update retracted bluemonday ver (trufflesecurity#3369)
  Fix git binary handling and add a smoke test (trufflesecurity#3379)
  fix(deps): update module google.golang.org/protobuf to v1.35.1 (trufflesecurity#3382)
  Added Cisco Meraki API Key detector (trufflesecurity#3367)
  improved the agora detector (trufflesecurity#3360)
  fix(deps): update module github.com/xanzy/go-gitlab to v0.110.0 (trufflesecurity#3376)
  fix(deps): update golang.org/x/exp digest to 225e2ab (trufflesecurity#3371)
  fix(deps): update module golang.org/x/net to v0.30.0 (trufflesecurity#3373)
  fix(deps): update module golang.org/x/crypto to v0.28.0 (trufflesecurity#3372)
  chore(deps): update sigstore/cosign-installer action to v3.7.0 (trufflesecurity#3368)
  fix(deps): update module cloud.google.com/go/storage to v1.44.0 (trufflesecurity#3366)
  fix(deps): update module github.com/schollz/progressbar/v3 to v3.16.1 (trufflesecurity#3365)
  [refactor] - Decouple Metrics From Cache Implementation (trufflesecurity#3355)
  fix(deps): update module github.com/snowflakedb/gosnowflake to v1.11.2 (trufflesecurity#3363)
  Updated Cosign Install URL (trufflesecurity#3364)
  fix(deps): update module github.com/jedib0t/go-pretty/v6 to v6.6.0 (trufflesecurity#3361)
  Added Pattern test cases for detectors (trufflesecurity#3354)
  remove size check (trufflesecurity#3351)
  fix(deps): update module go.mongodb.org/mongo-driver to v1.17.1 (trufflesecurity#3357)
  ...

# Conflicts:
#	go.sum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants